STOR-5615: Set the Durable Object retry policy per stub by apeacock1991 · Pull Request #7672 · cloudflare/workerd

apeacock1991 · 2026-10-09T10:46:25Z

Adds a retry option when creating a stub on a DurableObjectNamespace, replacing the binding-level retryPolicy from #7383.

const stub = env.MY_DO.get(id, {
  retry: { maxRetries: 5, timeoutMs: 10_000 },
});

const other = ctx.exports.MyDO.getByName("foo", {
  retry: { maxRetries: 0 },
});

The policy belongs to the returned stub, so each call site chooses its own. env bindings and ctx.exports behave the same.

maxRetries: an integer from 0 to 10, counting retries after the initial attempt. maxRetries: 4 means five attempts in total.
timeoutMs: an integer from 500 to 60000.
maxRetries and timeoutMs reject non-number values, including null, with a TypeError rather than coercing them. Other numbers outside the limits, including fractions, throw a RangeError. Omitted fields use the runtime's defaults.
Passing retry: null is equivalent to retry: {} and uses the runtime's defaults.
The option is experimental. Without the experimental compatibility flag it is ignored and absent from the public types. Workers with the flag get separate get()/getByName()/getExisting() registrations whose options dictionary has retry; other workers keep the dictionary without it, so the property is never read for them and neither a bad value nor a throwing getter can affect them.
With the flag, it is always validated, but the policy applies only while the DURABLE_OBJECT_RETRIES_USERLAND autogate is on. Turning the gate on changes retry behaviour, not which inputs throw.

The stub already carried its own policy (GlobalActorOutgoingFactory), so getImpl() now fills it from the options instead of from the namespace. UserDefinedRetryPolicy::maxAttempts and its constants are renamed to maxRetries to match the option. ActorRetryPolicy::maxAttempts() keeps its name, since it counts every attempt. The binding field, its copy in compiled-bindings.capnp and the namespace constructor parameter are removed. Embedders that pass a policy to that constructor need updating. Cap'n Proto needs the ordinals kept, so the fields become obsolete2 @2 and obsolete28 @28 (both :AnyPointer). Nothing set it: the Miniflare change that would have (cloudflare/workers-sdk#15872) has been closed.

ask-bonk · 2026-10-09T10:50:20Z

Since last review: 0 resolved, 0 still open, 0 new.
LGTM!

Not re-run: api-compat, tests, docs, compat-flags, design-simplicity, jsg-gc, kj-style, memory-safety, rust-first (no author changes in their files since the last review)

_{Reviewed commit: 01bb2e6d · github run}

ask-bonk · 2026-10-12T12:49:34Z

<title>502 Bad Gateway</title>

502 Bad Gateway

nginx

github run

ask-bonk · 2026-10-12T12:49:47Z

@apeacock1991 Bonk workflow failed. Check the logs for details.

View workflow run · To retry, trigger Bonk again.

ask-bonk · 2026-10-12T12:49:47Z

@apeacock1991 Bonk workflow failed. Check the logs for details.

View workflow run · To retry, trigger Bonk again.

ask-bonk · 2026-10-12T12:49:47Z

@apeacock1991 Bonk workflow failed. Check the logs for details.

View workflow run · To retry, trigger Bonk again.

maxmcd

If the stub is serialized, is the retry policy dropped? Does that matter?

Add a `retry` option to DurableObjectNamespace's get(), getByName() and getExisting(): env.MY_DO.get(id, { retry: { maxRetries: 5, timeoutMs: 10_000 } }) The policy belongs to the returned stub, so each call site chooses its own, and ctx.exports namespaces accept it the same way as env bindings. maxRetries must be an integer from 0 to 10 and timeoutMs an integer from 500 to 60000. Both fields reject non-number values, including null, with a TypeError rather than coercing them. Other numbers outside those limits, including fractions, throw a RangeError. Omitted fields use the runtime's defaults. Passing `retry: null` is equivalent to `retry: {}` and uses the runtime's defaults. The option is experimental: without the `experimental` compatibility flag it is absent from the public types and never read. Workers with the flag get separate get(), getByName() and getExisting() registrations whose options dictionary has the `retry` field; other workers keep the dictionary without it, so neither a bad value nor a throwing getter can affect them. With the flag, the option is always validated, but the policy applies only while the DURABLE_OBJECT_RETRIES_USERLAND autogate is on, so turning the gate on changes retry behaviour without changing which inputs throw. maxRetries counts retries after the initial attempt, so the UserDefinedRetryPolicy field and its constants are renamed from maxAttempts to match. ActorRetryPolicy::maxAttempts() keeps its name, since it counts every attempt. This replaces the binding-level retryPolicy from #7383, so that field, its copy in compiled-bindings.capnp and the namespace constructor parameter that carried it are removed. Cap'n Proto needs the ordinals kept, so the fields become obsolete2 and obsolete28.

clanky-the-clanker · 2026-10-16T08:51:10Z

🤖 AI code review

💬 Approved with comments — No blocking or warning findings. The only remaining item is a SUGGESTION to rename a stale test and its comment, which still describe the removed binding-level policy.

Findings: 1 suggestion

_{The decision is advisory: this comment never approves, blocks or requests changes on the pull request.}

🔧 Fix with your agent

Paste the following into your coding agent to investigate and fix these issues.

Fix the following code review issues found in pull request #7672.

Before making changes, review each issue and present a brief summary table:
- For each finding, state whether you agree, disagree, or need clarification
- If you disagree (e.g., the fix would require disproportionate changes
  for minimal ROI, or the finding is factually incorrect), explain why
- If you need to clarify something before deciding, ask those questions
- Then share your plan for which issues to tackle and in what order

## SUGGESTION (1)

### [Code Quality] Test name and comment still describe the removed binding-level retry policy; reword to refer to the stub.
- **File:** src/workerd/api/actor-fetch-retry-test.c++:1144
- **Issue:** Test name and comment still describe the removed binding-level retry policy; reword to refer to the stub.

📋 Code Quality: 💬 1 comment

The per-stub retry option looks correct. The only item is a stale test name and comment.

Recommendation: 💬 Optional Nits

[SUGGESTION] Test name and comment still describe the removed binding-level retry policy; reword to refer to the stub. (src/workerd/api/actor-fetch-retry-test.c++:1144)

View full review

src/workerd/api/actor-fetch-retry-test.c++:1144 and :1160-1161: the test is named "actor fetch uses the system default retry count when the binding sets none", and its comment says "No policy on the binding falls back to the system default". The policy now belongs to the stub, and this PR rewords the same idea in http.c++, http.h, worker-rpc.* and actor-call-retry.h. Reword this test to say "stub" as well.

Checked with no issues found:

Overloads. The 4-argument and 5-argument getImpl overloads don't make calls ambiguous.
Validation order. Values are validated before the autogate check, so invalid values throw whether or not the gate is on.
Rejected values. isIntegerInRange rejects NaN and fractions.
Flag gating. Workers without the experimental flag never read retry; the C++ tests cover this, including a throwing getter.
Removed binding field. No remaining readers of the binding field in C++ or Rust. The capnp ordinals are kept as AnyPointer placeholders.
Call sites. Constructor call sites are consistent, and jurisdiction() correctly stops passing a policy along.
Tests. Coverage includes C++ validation tests, gate-off and no-flag paths, the .wd-test for both env and ctx.exports, and a type test.

✅ All other checks passed

🔒 Security: ✅ LGTM

No security concerns. Input values are validated, and the option only takes effect with the experimental flag.

📖 AGENTS.md: ✅ LGTM

This PR doesn't change any AGENTS.md file, and no guidance needs updating. A suggestion about the length of the root AGENTS.md was dropped because it has nothing to do with this change.

⚡ Performance: ✅ LGTM

Validation is O(1) per stub creation, and copying the options is trivial.

📝 Documentation: ✅ LGTM

No documentation issues found.

📘 How the review decision works

[BLOCKING] findings (dangerous, exploitable or outage-risk problems) make the decision "significant concerns".
[WARNING] findings: any with production impact, or 4+ that each state a correctness, reliability, security or production impact → "minor issues"; otherwise → "approved with comments". Docs wording, style, test-typing, observability and process warnings don't count toward the 4.
[INFO], [SUGGESTION], [QUESTION] never count against the change.
Findings in test files and justified exceptions (e.g. // intentional:) are rated lower.
The decision is advisory: it never approves, blocks or requests changes on this pull request.

_{Reviewed commit: 01bb2e6d}

apeacock1991 requested review from a team as code owners October 9, 2026 10:46

ask-bonk Bot reviewed Oct 9, 2026

View reviewed changes

Comment thread src/workerd/server/workerd.capnp

Comment thread src/workerd/server/server.c++ Outdated

apeacock1991 force-pushed the apeacock/STOR-5615-do-retry-policy-owner branch from 54a66ca to c93799f Compare October 12, 2026 09:37

apeacock1991 requested a review from a team as a code owner October 12, 2026 09:37

apeacock1991 requested a review from petebacondarwin October 12, 2026 09:37

apeacock1991 changed the title ~~STOR-5615: Move the Durable Object retry policy to the namespace~~ STOR-5615: Set the Durable Object retry policy per stub Oct 12, 2026

ask-bonk Bot reviewed Oct 12, 2026

View reviewed changes

Comment thread src/workerd/api/actor.c++ Outdated

apeacock1991 force-pushed the apeacock/STOR-5615-do-retry-policy-owner branch from c93799f to 0e92560 Compare October 12, 2026 10:13

jamesopstad requested review from jamesopstad and removed request for petebacondarwin October 12, 2026 10:39

jamesopstad reviewed Oct 12, 2026

View reviewed changes

Comment thread src/workerd/api/actor.h Outdated

apeacock1991 force-pushed the apeacock/STOR-5615-do-retry-policy-owner branch from 0e92560 to 92896c8 Compare October 12, 2026 12:46

ask-bonk Bot reviewed Oct 12, 2026

View reviewed changes

Comment thread src/workerd/api/actor.h

jamesopstad approved these changes Oct 12, 2026

View reviewed changes

Comment thread src/workerd/api/actor-fetch-retry-test.c++

apeacock1991 force-pushed the apeacock/STOR-5615-do-retry-policy-owner branch 2 times, most recently from 7c45ce4 to cb865fa Compare October 13, 2026 16:34

maxmcd approved these changes Oct 14, 2026

View reviewed changes

Comment thread src/workerd/api/actor.h Outdated

Comment thread src/workerd/server/workerd.capnp

apeacock1991 force-pushed the apeacock/STOR-5615-do-retry-policy-owner branch 2 times, most recently from 64c72be to f220985 Compare October 14, 2026 19:18

ask-bonk Bot reviewed Oct 14, 2026

View reviewed changes

Comment thread src/workerd/api/actor.h

apeacock1991 force-pushed the apeacock/STOR-5615-do-retry-policy-owner branch 2 times, most recently from b325f92 to c0394c2 Compare October 15, 2026 09:53

ask-bonk Bot reviewed Oct 15, 2026

View reviewed changes

Comment thread src/workerd/api/actor.h

apeacock1991 force-pushed the apeacock/STOR-5615-do-retry-policy-owner branch from c0394c2 to 9565a40 Compare October 15, 2026 10:43

ask-bonk Bot reviewed Oct 15, 2026

View reviewed changes

Comment thread src/workerd/api/actor-fetch-retry-test.c++

apeacock1991 force-pushed the apeacock/STOR-5615-do-retry-policy-owner branch 3 times, most recently from 18e72d1 to 91a11d3 Compare October 15, 2026 15:57

apeacock1991 force-pushed the apeacock/STOR-5615-do-retry-policy-owner branch from 91a11d3 to 01bb2e6 Compare October 16, 2026 08:48

apeacock1991 merged commit e0bf624 into main Oct 16, 2026
37 of 39 checks passed

apeacock1991 deleted the apeacock/STOR-5615-do-retry-policy-owner branch October 16, 2026 10:28

Conversation

apeacock1991 commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ask-bonk Bot commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ask-bonk Bot commented Oct 12, 2026

502 Bad Gateway

Uh oh!

ask-bonk Bot commented Oct 12, 2026

Uh oh!

ask-bonk Bot commented Oct 12, 2026

Uh oh!

ask-bonk Bot commented Oct 12, 2026

Uh oh!

Uh oh!

Uh oh!

maxmcd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

clanky-the-clanker Bot commented Oct 16, 2026

🤖 AI code review

📋 Code Quality: 💬 1 comment

🔒 Security: ✅ LGTM

📖 AGENTS.md: ✅ LGTM

⚡ Performance: ✅ LGTM

📝 Documentation: ✅ LGTM

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

apeacock1991 commented Oct 9, 2026 •

edited

Loading

ask-bonk Bot commented Oct 9, 2026 •

edited

Loading